Skip to content

[Backend] Fix issue refunding uncompleted payments#6094

Merged
tvdeyen merged 2 commits intosolidusio:mainfrom
jtapia:bugfix/admin-payment-action-options
Apr 9, 2025
Merged

[Backend] Fix issue refunding uncompleted payments#6094
tvdeyen merged 2 commits intosolidusio:mainfrom
jtapia:bugfix/admin-payment-action-options

Conversation

@jtapia
Copy link
Contributor

@jtapia jtapia commented Jan 31, 2025

Summary

  • Remove Refund button for uncompleted payments
  • Remove Credit button for payment on void and invalid state

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@jtapia jtapia requested a review from a team as a code owner January 31, 2025 22:56
@github-actions github-actions bot added the changelog:solidus_backend Changes to the solidus_backend gem label Jan 31, 2025
@jtapia
Copy link
Contributor Author

jtapia commented Jan 31, 2025

In favor of #2932

@codecov
Copy link

codecov bot commented Jan 31, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.64%. Comparing base (a57f0c9) to head (4b11b05).
Report is 12 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6094      +/-   ##
==========================================
- Coverage   88.82%   86.64%   -2.19%     
==========================================
  Files         835      514     -321     
  Lines       18149    11891    -6258     
==========================================
- Hits        16121    10303    -5818     
+ Misses       2028     1588     -440

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jtapia jtapia changed the title Fix issue refunding uncompleted payments [Backend] Fix issue refunding uncompleted payments Jan 31, 2025
@fthobe
Copy link
Contributor

fthobe commented Feb 1, 2025 •
edited
Loading

Hey @jtapia thank you for picking this up again after so much time. Seems cov was flaky. Thank you so much!

@jtapia jtapia force-pushed the bugfix/admin-payment-action-options branch from 7e914f7 to 4ce6200 Compare February 2, 2025 21:32
@jtapia
Copy link
Contributor Author

jtapia commented Feb 2, 2025

@fthobe I believe the broken test is a flaky one. Could someone help me re-running the process on CircleCI?

@kennyadsl
Copy link
Member

kennyadsl commented Feb 3, 2025

Hey @jtapia thanks for contributing. Can you put a :flaky on the spec definition that fails? This way we will be covered next time it happens. Thanks!

See

solidus/backend/spec/features/admin/users_spec.rb

Line 471 in ca1ae4d

it 'prefills the customer addresses with the user addresses', :flaky do
and

solidus/core/lib/spree/testing_support/flaky.rb

Line 15 in ca1ae4d

config.around(:each, :flaky) do |example|
.

Thanks!

@jtapia jtapia force-pushed the bugfix/admin-payment-action-options branch from 4ce6200 to 6a19837 Compare February 4, 2025 00:49
@jtapia
Copy link
Contributor Author

jtapia commented Feb 4, 2025 •
edited
Loading

@kennyadsl thanks, I have made the changes here: https://github.com/solidusio/solidus/pull/6094/files#diff-b8b2f84275223ed3dc20600049f3eba20911f6e12ed489c34be875590e272599R76, I noticed that codecov is still showing a decrease in coverage, even though it was only a view code change. Is there anything might be missing?

@kennyadsl
Copy link
Member

kennyadsl commented Feb 4, 2025

@jtapia thanks! If the view contains logic, it's perfectly legit for codecov to compute the coverage there. It means that we added some conditional paths that are not currently tested.

@tvdeyen tvdeyen force-pushed the bugfix/admin-payment-action-options branch from 6a19837 to bf82c87 Compare February 19, 2025 16:54
tvdeyen
tvdeyen reviewed Feb 19, 2025
View reviewed changes
@jtapia jtapia force-pushed the bugfix/admin-payment-action-options branch from bf82c87 to 4b11b05 Compare February 23, 2025 17:45
@tvdeyen tvdeyen force-pushed the bugfix/admin-payment-action-options branch from 4b11b05 to a3199c1 Compare February 24, 2025 08:28
@fthobe
Copy link
Contributor

fthobe commented Feb 24, 2025

@tvdeyen we just closed an I believe 4 year old ticket with that 😅😅😅 @jtapia thank you so much for picking that up again.

@fthobe
Copy link
Contributor

fthobe commented Apr 7, 2025

@kennyadsl ready to be merged

@tvdeyen tvdeyen force-pushed the bugfix/admin-payment-action-options branch from a3199c1 to 3670ead Compare April 7, 2025 18:13
@tvdeyen tvdeyen enabled auto-merge April 7, 2025 18:13
@fthobe
Copy link
Contributor

fthobe commented Apr 7, 2025

@tvdeyen is there anything left to do or is this ok?

@tvdeyen tvdeyen disabled auto-merge April 7, 2025 20:04
@tvdeyen
Copy link
Member

tvdeyen commented Apr 7, 2025

@tvdeyen is there anything left to do or is this ok?

A second approval from @solidusio/core-team

@fthobe
Copy link
Contributor

fthobe commented Apr 9, 2025

@tvdeyen can we merge this given that requirements have been met. It's open for too much time.

@tvdeyen tvdeyen merged commit ceadf35 into solidusio:main Apr 9, 2025
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tvdeyen tvdeyen tvdeyen approved these changes

@jarednorman jarednorman jarednorman approved these changes

+1 more reviewer

@fthobe fthobe fthobe left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

changelog:solidus_admin changelog:solidus_backend Changes to the solidus_backend gem

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jtapia @fthobe @kennyadsl @tvdeyen @jarednorman